Skip to content

fix(mcp): don't blank the MCP tools page when tool discovery fails#5816

Merged
waleedlatif1 merged 2 commits into
stagingfrom
fix/mcp-tools-page-degradation
Jul 21, 2026
Merged

fix(mcp): don't blank the MCP tools page when tool discovery fails#5816
waleedlatif1 merged 2 commits into
stagingfrom
fix/mcp-tools-page-degradation

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • A tool-discovery failure (one slow/failing server — e.g. a stalled streamable-HTTP transport timing out) replaced the entire MCP tools server list with a centered error banner, hiding every server including healthy/authorizable ones.
  • Root cause: const error = toolsError || serversError gated the whole list, conflating a tool-discovery failure with a server-list-load failure. Either one blanked the page.
  • Fix: gate the full-list replacement on serversError only (the list genuinely failing to load). When the servers loaded, always render the list — each row already surfaces its own discovery state via toolsStateByServer (per-row status), with a small non-blocking notice above the list when discovery had issues.
  • This restores the per-server isolation intent: one bad server can't hide the others, and you can still act on the list (reconnect an OAuth server, retry, etc.).

Type of Change

  • Bug fix (UX resilience)

Testing

  • Type-checks + biome clean.
  • Audited the full discovery lifecycle (hook useMcpToolsQuery per-server useQueries isolation, the discover route, the service single/all-server paths, and every render state): page never blanks on a tool-discovery failure in any of loading / no-servers / servers+toolsError / serversError / detail-view; per-server isolation intact; no other code hides the list; no unused vars or regressions.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

A tool-discovery error (one slow/failing server, e.g. a stalled transport)
replaced the entire server list with an error banner. Gate the full-list
replacement on serversError (the list genuinely failing to load) only; when the
servers loaded, always render the list — each row already surfaces its own
discovery state via toolsStateByServer, with a non-blocking notice above the
list. Restores graceful degradation so one bad server can't hide the others.
@vercel

vercel Bot commented Jul 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 21, 2026 8:43pm

Request Review

@cursor

cursor Bot commented Jul 21, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Settings UI error-handling only; no auth, API, or data-model changes.

Overview
Fixes MCP settings so a tool-discovery failure no longer replaces the whole server list with a full-page error. Full-page blocking state now uses serversError only; toolsError and per-server failures from toolsStateByServer no longer hide healthy servers.

When the list still loads, users see a non-blocking notice if any discovery failed, and each ServerListItem can show a per-row discoveryError (with error styling) instead of misleading “0 tools” when stored connection status hasn’t caught up yet.

Reviewed by Cursor Bugbot for commit 8ad1c31. Configure here.

@greptile-apps

greptile-apps Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR keeps the MCP server list usable when tool discovery fails. The main changes are:

  • Blocks the full page only when the server list fails to load.
  • Shows a non-blocking notice for discovery failures.
  • Displays live discovery errors on affected server rows.

Confidence Score: 5/5

This looks safe to merge.

  • The updated per-server error handling resolves the reported partial-discovery behavior.
  • No blocking issues were found in the changed code.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/settings/components/mcp/mcp.tsx Separates server-list errors from tool-discovery errors and surfaces partial failures without hiding the server list.

Reviews (2): Last reviewed commit: "fix(mcp): surface partial discovery fail..." | Re-trigger Greptile

Comment thread apps/sim/app/workspace/[workspaceId]/settings/components/mcp/mcp.tsx Outdated
When one server succeeds and another fails, the aggregate toolsError is
suppressed (data exists), so the failure was hidden. Now: the notice renders on
ANY per-server discovery error (not just all-fail), and each failed row surfaces
its live discovery error instead of reading as '0 tools' before its stored
status catches up.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8ad1c31. Configure here.

@waleedlatif1
waleedlatif1 merged commit d35bc77 into staging Jul 21, 2026
20 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/mcp-tools-page-degradation branch July 21, 2026 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant